-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added AWS_STS_REGIONAL_ENDPOINTS flag/annotation #55
Conversation
0d09994
to
454d3b4
Compare
@@ -323,11 +337,6 @@ func (m *Modifier) Handle(w http.ResponseWriter, r *http.Request) { | |||
body = data | |||
} | |||
} | |||
if len(body) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason this was removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, this error wasn't helpful and didn't return an AdmissionReview
I could add it back and just return an actual AdmissionReview
with a more helpful error message instead of attempting to process the empty body
454d3b4
to
e95bb68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than I think the go-jose library isn't actually used, this PR looks great, thank you Micah! :)
pod := &corev1.Pod{} | ||
err = yaml.Unmarshal(data, pod) | ||
return pod, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice test cleanup and separation of fixtures into testdata. ++
e95bb68
to
aaab99f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ship it :)
IRSA in a private cluster requires the use of STS VPC endpoints. However, the most AWS SDKs use the global STS endpoint by default for the STS `AssumeRoleWithWebIdentity` call, which bypasses the STS VPC endpoint (and fails in a private cluster). To make this work correctly, we may need to explicitly instruct the SDK to use the regional STS endpoint. Usually this is done by passing some environment variables: ```yaml - env: - name: AWS_REGION value: <REGION> - name: AWS_STS_REGIONAL_ENDPOINTS value: regional ``` Relevant info: aws/amazon-eks-pod-identity-webhook#55 (Note: `eks.amazonaws.com/sts-regional-endpoints` doesn't appear to be supported yet in EKS) https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_sts_vpce.html#id_credentials_sts_vpce_create
Issue #, if available:
N/A
Description of changes:
The default for most AWS v1 SDKs is to use the global STS endpoint (
https://sts.amazonaws.com
, hosted inus-east-1
) when talking to STS, resulting in a cross-region call for anything not inus-east-1
just to get credentials. (some v2 SDKs such as Java v2 use regional). The way to force AWS SDKs to use the regional STS endpoint is to add theAWS_STS_REGIONAL_ENDPOINTS
environment variable and set the value toregional
.For applications running in Kubernetes pods using this webhook, this setting can now be automatically configured by adding the ServiceAccount annotation
eks.amazonaws.com/sts-regional-endpoints
with a value of"true"
, or if operating this webhook on a cluster, setting the webhook flag--sts-regional-endpoint=true
.If the webhook's flag value is set to regional (
--sts-regional-endpoint=true
), but the environment variable is not desired, individual ServiceAccounts can opt-out by setting the annotation toeks.amazonaws.com/sts-regional-endpoints: "false"
.Note that IAM administrators can disable regional STS, and any applications that attempt to use a regional STS endpoint on a disabled region will get an error.